INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values #256

timgraham · 2025-02-11T00:22:51Z

Most of the work for this is adapting Django's test suite: mongodb-forks/django#15

Jibola · 2025-02-24T20:38:52Z

.github/workflows/test-python.yml

        with:
          repository: 'mongodb-forks/django'
-          ref: 'mongodb-5.1.x'
+          ref: 'objectid-no-int'


will this branch get merged into both 5.1 and 5.0?

Of course. Whenever you see a change like this is just means we needed to use a temporary staging branch until the patch is merged.

aclark4life

LGTM, thanks

aclark4life · 2025-02-28T21:15:14Z

@timgraham FYI I'm not sure why, but this PR makes DRF behave worse e.g. before this PR was merged (and with changes made in: encode/django-rest-framework#9656) I get a reasonable test failure:

(.venv) alex.clark@M-FD7GQ2QTFM django-mongodb-cli % dj repo test django-rest-framework test_search_field_with_custom_lookup
Copying test/settings/settings_drf.py to src/django-rest-framework/tests/conftest.py
Copying test/apps/apps_drf.py to src/django-rest-framework/tests/mongo_apps.py
Running ./runtests.py test_search_field_with_custom_lookup
/Users/alex.clark/Developer/django-mongodb-cli/.venv/lib/python3.13/site-packages/coreapi/utils.py:5: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
======================================================================================================= test session starts =======================================================================================================
platform darwin -- Python 3.13.2, pytest-7.4.4, pluggy-1.5.0
django: version: 5.1.7.dev20250225003443
rootdir: /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework
configfile: setup.cfg
plugins: django-4.10.0, cov-4.1.0
collected 1558 items / 1557 deselected / 1 selected                                                                                                                                                                               

tests/test_filters.py F                                                                                                                                                                                                     [100%]

============================================================================================================ FAILURES =============================================================================================================
_____________________________________________________________________________________ SearchFilterTests.test_search_field_with_custom_lookup ______________________________________________________________________________________
tests/test_filters.py:225: in test_search_field_with_custom_lookup
    assert response.data == [
E   AssertionError: assert [{'id': Objec...text': 'abc'}] == [{'id': 1, 't...'title': 'z'}]
E     At index 0 diff: {'id': ObjectId('67c223aaa0bcd53f900ded34'), 'title': 'z', 'text': 'abc'} != {'id': 1, 'title': 'z', 'text': 'abc'}
E     Use -v to get more diff
======================================================================================================== warnings summary =========================================================================================================
rest_framework/fields.py:995
  /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework/rest_framework/fields.py:995: UserWarning: max_value should be an integer or Decimal instance.
    warnings.warn("max_value should be an integer or Decimal instance.")

rest_framework/fields.py:997
  /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework/rest_framework/fields.py:997: UserWarning: min_value should be an integer or Decimal instance.
    warnings.warn("min_value should be an integer or Decimal instance.")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
===================================================================================================== short test summary info =====================================================================================================
FAILED tests/test_filters.py::SearchFilterTests::test_search_field_with_custom_lookup - AssertionError: assert [{'id': Objec...text': 'abc'}] == [{'id': 1, 't...'title': 'z'}]
========================================================================================= 1 failed, 1557 deselected, 2 warnings in 5.15s =======================================================================================

With current main I get:


(.venv) alex.clark@M-FD7GQ2QTFM django-mongodb-cli % dj repo test django-rest-framework test_search_field_with_custom_lookup
Copying test/settings/settings_drf.py to src/django-rest-framework/tests/conftest.py
Copying test/apps/apps_drf.py to src/django-rest-framework/tests/mongo_apps.py
Running ./runtests.py test_search_field_with_custom_lookup
/Users/alex.clark/Developer/django-mongodb-cli/.venv/lib/python3.13/site-packages/coreapi/utils.py:5: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
======================================================================================================= test session starts =======================================================================================================
platform darwin -- Python 3.13.2, pytest-7.4.4, pluggy-1.5.0
django: version: 5.1.7.dev20250225003443
rootdir: /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework
configfile: setup.cfg
plugins: django-4.10.0, cov-4.1.0
collected 1558 items / 1557 deselected / 1 selected                                                                                                                                                                               

tests/test_filters.py E                                                                                                                                                                                                     [100%]

============================================================================================================= ERRORS ==============================================================================================================
____________________________________________________________________________ ERROR at setup of SearchFilterTests.test_search_field_with_custom_lookup _____________________________________________________________________________
../../.venv/lib/python3.13/site-packages/pytest_django/plugin.py:584: in _django_setup_unittest
    request.getfixturevalue("django_db_setup")
../../.venv/lib/python3.13/site-packages/pytest_django/fixtures.py:144: in django_db_setup
    db_cfg = setup_databases(
../django/django/test/utils.py:206: in setup_databases
    connection.creation.create_test_db(
../django/django/db/backends/base/creation.py:78: in create_test_db
    call_command(
../django/django/core/management/__init__.py:194: in call_command
    return command.execute(*args, **defaults)
../django/django/core/management/base.py:459: in execute
    output = self.handle(*args, **options)
../django/django/core/management/base.py:107: in wrapper
    res = handle_func(*args, **kwargs)
../django/django/core/management/commands/migrate.py:384: in handle
    emit_post_migrate_signal(
../django/django/core/management/sql.py:52: in emit_post_migrate_signal
    models.signals.post_migrate.send(
../django/django/dispatch/dispatcher.py:189: in send
    response = receiver(signal=self, sender=sender, **named)
../django/django/contrib/sites/management.py:37: in create_default_site
    ).save(using=using)
../django/django/db/models/base.py:892: in save
    self.save_base(
../django/django/db/models/base.py:998: in save_base
    updated = self._save_table(
../django/django/db/models/base.py:1130: in _save_table
    updated = self._do_update(
../django/django/db/models/base.py:1174: in _do_update
    filtered = base_qs.filter(pk=pk_val)
../django/django/db/models/query.py:1476: in filter
    return self._filter_or_exclude(False, args, kwargs)
../django/django/db/models/query.py:1494: in _filter_or_exclude
    clone._filter_or_exclude_inplace(negate, args, kwargs)
../django/django/db/models/query.py:1501: in _filter_or_exclude_inplace
    self._query.add_q(Q(*args, **kwargs))
../django/django/db/models/sql/query.py:1609: in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
../django/django/db/models/sql/query.py:1641: in _add_q
    child_clause, needed_inner = self.build_filter(
../django/django/db/models/sql/query.py:1555: in build_filter
    condition = self.build_lookup(lookups, col, value)
../django/django/db/models/sql/query.py:1385: in build_lookup
    lookup = lookup_class(lhs, rhs)
../django/django/db/models/lookups.py:30: in __init__
    self.rhs = self.get_prep_lookup()
../django/django/db/models/lookups.py:369: in get_prep_lookup
    return super().get_prep_lookup()
../django/django/db/models/lookups.py:88: in get_prep_lookup
    return self.lhs.output_field.get_prep_value(self.rhs)
../django-mongodb-backend/django_mongodb_backend/fields/auto.py:25: in get_prep_value
    return self.to_python(value)
../django-mongodb-backend/django_mongodb_backend/fields/objectid.py:31: in to_python
    raise exceptions.ValidationError(
E   django.core.exceptions.ValidationError: ['“1” is not a valid Object Id.']
======================================================================================================== warnings summary =========================================================================================================
rest_framework/fields.py:995
  /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework/rest_framework/fields.py:995: UserWarning: max_value should be an integer or Decimal instance.
    warnings.warn("max_value should be an integer or Decimal instance.")

rest_framework/fields.py:997
  /Users/alex.clark/Developer/django-mongodb-cli/src/django-rest-framework/rest_framework/fields.py:997: UserWarning: min_value should be an integer or Decimal instance.
    warnings.warn("min_value should be an integer or Decimal instance.")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
===================================================================================================== short test summary info =====================================================================================================
ERROR tests/test_filters.py::SearchFilterTests::test_search_field_with_custom_lookup - django.core.exceptions.ValidationError: ['“1” is not a valid Object Id.']
========================================================================================== 1557 deselected, 2 warnings, 1 error in 5.09s ==========================================================================================
Exception ignored in: <django.core.management.base.OutputWrapper object at 0x112c69720>
Traceback (most recent call last):
  File "/Users/alex.clark/Developer/django-mongodb-cli/src/django/django/core/management/base.py", line 171, in flush
    self._out.flush()
ValueError: I/O operation on closed file.
Exception ignored in: <django.core.management.base.OutputWrapper object at 0x112ee0a00>
Traceback (most recent call last):
  File "/Users/alex.clark/Developer/django-mongodb-cli/src/django/django/core/management/base.py", line 171, in flush
    self._out.flush()
ValueError: I/O operation on closed file.

The former can be fixed, but the latter may be a regression since it appears to prevent DRF from configuring it's own serializer_field_mapping.

timgraham · 2025-02-28T22:12:17Z

Solution: https://django-mongodb-backend.readthedocs.io/en/latest/howto/contrib-apps.html#contrib-sites
https://github.com/encode/django-rest-framework/blob/0e1c7d3613905a8f9db69abb82f883e22e967119/tests/conftest.py#L28

timgraham force-pushed the objectid-no-int branch from 68c026b to c280480 Compare February 11, 2025 02:39

timgraham mentioned this pull request Feb 11, 2025

adapt tests for ObjectIdAutoField mongodb-forks/django#15

Merged

timgraham force-pushed the objectid-no-int branch 4 times, most recently from aea0911 to 5e98655 Compare February 14, 2025 21:11

timgraham changed the title ~~prohibit int in ObjectIdAutoField~~ INTPYTHON-393 Remove ObjectIdAutoField acceptance of integers Feb 21, 2025

timgraham force-pushed the objectid-no-int branch from 2e4bc3c to 7794da2 Compare February 21, 2025 15:04

timgraham marked this pull request as ready for review February 21, 2025 15:28

timgraham requested review from Jibola and aclark4life February 21, 2025 15:28

timgraham changed the title ~~INTPYTHON-393 Remove ObjectIdAutoField acceptance of integers~~ INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values Feb 24, 2025

timgraham force-pushed the objectid-no-int branch 2 times, most recently from 869df34 to e6f36fe Compare February 24, 2025 20:24

Jibola reviewed Feb 24, 2025

View reviewed changes

aclark4life approved these changes Feb 24, 2025

View reviewed changes

timgraham force-pushed the objectid-no-int branch from e6f36fe to a67ba32 Compare February 25, 2025 00:27

INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values

59ace00

timgraham force-pushed the objectid-no-int branch from a67ba32 to 59ace00 Compare February 25, 2025 00:28

timgraham merged commit 59ace00 into mongodb:main Feb 25, 2025
13 checks passed

timgraham deleted the objectid-no-int branch February 25, 2025 01:02

blink1073 temporarily deployed to release February 25, 2025 05:35 — with GitHub Actions Inactive

blink1073 temporarily deployed to release February 25, 2025 05:37 — with GitHub Actions Inactive

blink1073 temporarily deployed to release February 25, 2025 05:39 — with GitHub Actions Inactive

blink1073 had a problem deploying to release February 25, 2025 05:39 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values #256

INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values #256

Uh oh!

timgraham commented Feb 11, 2025 •

edited

Loading

Uh oh!

Jibola Feb 24, 2025

Uh oh!

timgraham Feb 24, 2025

Uh oh!

aclark4life left a comment

Uh oh!

Uh oh!

aclark4life commented Feb 28, 2025 •

edited

Loading

Uh oh!

timgraham commented Feb 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values #256

INTPYTHON-393 Remove ObjectIdAutoField acceptance of integer values #256

Uh oh!

Conversation

timgraham commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jibola Feb 24, 2025

Choose a reason for hiding this comment

Uh oh!

timgraham Feb 24, 2025

Choose a reason for hiding this comment

Uh oh!

aclark4life left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aclark4life commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timgraham commented Feb 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timgraham commented Feb 11, 2025 •

edited

Loading

aclark4life commented Feb 28, 2025 •

edited

Loading